stream: allow null as second arg in Transform callback by galaxy4276 · Pull Request #62803 · nodejs/node

galaxy4276 · 2026-04-18T13:12:12Z

What

The docs say passing a value as the second argument to the transform callback is equivalent to calling transform.push(value). That implies callback(null, null) should push null — ending the readable side — just like this.push(null); callback() does. It doesn't.

Bug

const { Transform } = require('stream');

const t = new Transform({
  transform(chunk, encoding, callback) {
    callback(null, null); // intend to signal EOF
  },
});

t.on('end', () => console.log('ended')); // never fires
t.write('hello');
t.end();

The 'end' event never fires. The stream hangs.

Root cause — lib/internal/streams/transform.js:

// Before
if (val != null) {   // loose equality: null == null → true → blocks push(null)
  this.push(val);
}

val != null uses loose equality, so both null and undefined fail the check. this.push(null) is never called, state.ended stays false, and 'end' is never emitted.

Fix

// After
if (val !== undefined) {  // only block undefined; null passes through
  this.push(val);
}

Now callback(null, null) reaches this.push(null), which sets state.ended = true and eventually emits 'end' once the buffer drains — matching what the docs describe.

Behavior after fix

const t = new Transform({
  transform(chunk, encoding, callback) {
    callback(null, null); // equivalent to: this.push(null); callback()
  },
});

t.on('end', () => console.log('ended')); // fires correctly now
t.write('hello');
t.end();

Potential concern

This is technically a behavior change. Code that relied on callback(null, null) not ending the stream should use callback() (no second argument) instead. I think this is the right call given the docs, but open to pushback — especially on whether this warrants a semver-minor label or a deprecation note before changing.

nodejs-github-bot · 2026-04-18T13:12:18Z

Review requested:

@nodejs/streams

galaxy4276 · 2026-04-18T13:19:01Z

Looking at the history, val != null has been in transform.js since the file moved to internal/streams (~2020), so this isn't a regression — it's been silently broken for a long time. The loose equality check was likely written to block undefined (when no second arg is passed), but it incidentally also blocks null, which is the EOF sentinel.

The fix changes to val !== undefined, which only blocks undefined and lets null through to this.push(null) — matching what the docs describe:

"If a second argument is passed to the callback, it will be forwarded on to the transform.push() method"

In practice, callback(null, null) to signal EOF is uncommon — most users write this.push(null); callback() — so the blast radius of this change should be small. There's also no legitimate use case for callback(null, null) meaning "push nothing and continue", since null is the EOF sentinel in both buffer and object mode.

That said, I'm uncertain whether this is semver-patch (bug fix aligning with docs) or semver-minor (observable behavior change). Would appreciate guidance on the right label and whether a CHANGELOG note is needed.

nodejs-github-bot · 2026-04-18T15:00:52Z

CI: https://ci.nodejs.org/job/node-test-pull-request/72759/

codecov · 2026-04-18T16:30:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (31b9e60) to head (3040233).
⚠️ Report is 479 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #62803      +/-   ##
==========================================
- Coverage   89.70%   89.68%   -0.02%     
==========================================
  Files         706      706              
  Lines      218288   218222      -66     
  Branches    41782    41766      -16     
==========================================
- Hits       195806   195714      -92     
- Misses      14394    14412      +18     
- Partials     8088     8096       +8

Files with missing lines	Coverage Δ
lib/internal/streams/transform.js	`98.52% <100.00%> (ø)`

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

galaxy4276 · 2026-04-18T18:46:36Z

@lpinca @ronag The only change since your review is fixing a capitalized-comments lint error on line 21.
Would you mind re-approving?

mcollina

This should have a pass of citgm before landing.

I expect a significant breakage in the ecosystem.

galaxy4276 · 2026-04-19T08:11:16Z

This should have a pass of citgm before landing.

I expect a significant breakage in the ecosystem.

Thank you for your review.
As expected, since I'm changing the internal implementation, testing will be necessary.
I'll conduct stability testing through citgm and attach the results.

galaxy4276 · 2026-04-19T14:07:17Z

citgm run — results

TL;DR: Full citgm-all + extended stream subset + pattern audit show zero ecosystem regressions.
One pre-existing failure (JSONStream) is fixed by this change.
A semantic differential test confirms the behavior change is real but is not used by any library surveyed.

Environment

Field	Value
Platform	macOS Tahoe 26.4.1 (arm64)
Toolchain	Apple clang 21.0.0 / Xcode 26.4.1
Baseline	`58a8e1da5d` (`main`)
PR HEAD	`4aefb0790b`
citgm	v10.0.1, `-j 4`

Full citgm-all (72 modules)

	Baseline	PR
Pass	52	52
Fail (all pre-existing / env)	20	20
Regressions	—	0

Pre-existing failures (identical on both binaries)

@babel/core · @clinic/doctor · @yarnpkg/cli · ava · bufferutil · cheerio · commander · fastify · jose · jquery · koa · lru-cache · mime · nan · node-gyp · path-to-regexp · pino · prom-client · rewire · undici

All fail with the same error on both binaries (yarn resolution, native addon build on Apple clang 21, environment/flake). Spot-checked @babel/core: identical YN0060/YN0002 yarn errors baseline vs PR.

Extended stream subset (17 modules not in citgm lookup)

Ran against highland, duplexify, mississippi, parallel-transform, stream-to-promise, stream-each, stream-shift, get-stream, byline, JSONStream, ndjson, through, event-stream, from2, pump-file-to-s3, duplex-stream, multipipe.

	Baseline	PR
Pass	9	10
Fail (pre-existing)	8	7
Regressions	—	0
Fixes	—	1 (`JSONStream`)

Pattern audit

Grepped (callback\|cb\|done\|next)\s*\(\s*null\s*,\s*null across:

Top 50 most-downloaded npm packages
24 stream-centric libraries (readable-stream, through2, pump, pumpify, duplexify, split2, parallel-transform, …)

Hits outside Transform context: webpack (7), undici (2), mongoose (8) — all in unrelated async callbacks (resolver cache, dispatcher, cursor), not Transform._transform. None affected by this PR.

Hits inside Transform context: 0.

Semantic differential

Confirmed the change's behavior is observable with a focused test:

const t = new Transform({
  transform(chunk, enc, cb) {
    if (chunk.toString() === 'stop') cb(null, null);
    else cb(null, chunk);
  },
});
t.on('data', (d) => received.push(d));
t.write('a'); t.write('stop'); t.write('b'); t.end();

Node	Behavior
Baseline	`received = ['a', 'b']` (`'stop'` swallowed)
PR	`received = ['a']` + `ERR_STREAM_PUSH_AFTER_EOF` on the subsequent `write('b')`

So the change is a real semantic change — but we found no library in the tested universe that relies on it. Applications using callback(null, null) as an intentional no-op with subsequent writes would now see ERR_STREAM_PUSH_AFTER_EOF. This matches the documented intent of the callback (push the value passed) and the fix in this PR.

Conclusion & SemVer recommendation

citgm-all: 0 regressions across 72 modules
Extended stream subset: 0 regressions, 1 fix (JSONStream)
Pattern audit: 0 Transform-context callback(null, null) call sites in the surveyed ecosystem

Given the pattern is not observed in the ecosystem but is a runtime-visible change, I'd suggest semver-minor with a release-note call-out — conservative given the theoretical crash path, but supported by the zero-breakage evidence. Happy to relabel semver-patch if collaborators think the release note is sufficient.

Alternatives considered

Land as semver-minor with release note (recommended): documents the fix, warns the ERR_STREAM_PUSH_AFTER_EOF path for code that mis-used the callback.
Deprecation warning cycle first: emit process.emitWarning on callback(null, null) for one release, then flip behavior. Overkill given 0 ecosystem hits.
Opt-in flag: { strictNullPush: true } option on Transform. Adds API surface without evidence of need.
semver-patch with release note: defensible given zero detected breakage; trades collaborator caution for simplicity.

Raw artifacts

Available on request — stdout logs, per-module TAP, and pattern-hits file can be uploaded to a Gist if useful.

Notes

citgm-all ran in ~14 min per binary on an M4 Pro with 4-way concurrency; extended subset ran in ~10 min per binary.
node-test-citgm on Jenkins would be a stronger datapoint; this local run is offered as a starting point. Happy to iterate if additional coverage is requested.

callback(null, null) in a Transform._transform method should be equivalent to calling this.push(null) followed by callback(), ending the readable side of the stream. Previously val != null blocked the push because null == null is true in loose equality, so push(null) was never called and the stream never emitted 'end'. Change the guard from `val != null` to `val !== undefined` so that null passes through to this.push(), which sets state.ended and eventually emits 'end', matching documented behavior. Fixes: nodejs#62769

galaxy4276 · 2026-04-19T15:36:23Z

This should have a pass of citgm before landing.

I expect a significant breakage in the ecosystem.

@mcollina I ran citgm locally on macOS (arm64) and have attached the results above. Please take a look whenever you get a chance — no rush at all. Hope you have a great day!

mcollina

lgtm

mcollina · 2026-04-20T08:00:05Z

lowered to semver-minor with a "baking for lts" label.

nodejs-github-bot · 2026-04-20T08:33:03Z

CI: https://ci.nodejs.org/job/node-test-pull-request/72794/

galaxy4276 · 2026-04-20T19:33:40Z

@lpinca @ronag
Thanks for the previous review feedback.
I've completed the regression testing pass with citgm as suggested, and the results are detailed in the comments above.
There are no additional changes beyond what you've already reviewed.

I'd really appreciate it if you could take another look and re-approve whenever you have a moment.
Thanks so much for your continued support!

Have a great day!

trivikr · 2026-05-21T15:09:13Z

Coming from #62769, looks like this is ready for landing.
The baking-for-lts label is for release team as per docs.

@lpinca / @ronag can you provide your approval and add commit-queue label?

trivikr · 2026-05-21T15:28:20Z

Fixes: #62769

nodejs-github-bot · 2026-05-31T09:11:33Z

Commit Queue failed

- Loading data for nodejs/node/pull/62803 ✔ Done loading data for nodejs/node/pull/62803 ----------------------------------- PR info ------------------------------------ Title stream: allow null as second arg in Transform callback (#62803) Author eungi <deveungi@gmail.com> (@galaxy4276) Branch galaxy4276:stream/fix-transform-callback-null -> nodejs:main Labels stream, semver-minor, baking-for-lts, needs-ci Commits 1 - stream: allow null as second arg in Transform callback Committers 1 - galaxy4276 <deveungi@gmail.com> PR-URL: https://github.com/nodejs/node/pull/62803 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/62803 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 18 Apr 2026 13:12:12 GMT ✔ Approvals: 3 ✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/62803#pullrequestreview-4134963199 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/62803#pullrequestreview-4134589432 ✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/62803#pullrequestreview-4138192639 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-04-20T08:33:03Z: https://ci.nodejs.org/job/node-test-pull-request/72794/ - Querying data for job/node-test-pull-request/72794/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 62803 From https://github.com/nodejs/node * branch refs/pull/62803/merge -> FETCH_HEAD ✔ Fetched commits as 040c51c98f4f..3040233e7cef -------------------------------------------------------------------------------- [main a183368f0a] stream: allow null as second arg in Transform callback Author: galaxy4276 <deveungi@gmail.com> Date: Sun Apr 19 23:13:42 2026 +0900 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-stream-transform-callback-null.js ✔ Patches applied -------------------------------------------------------------------------------- --------------------------------- New Message ---------------------------------- stream: allow null as second arg in Transform callback

callback(null, null) in a Transform._transform method should be
equivalent to calling this.push(null) followed by callback(), ending
the readable side of the stream. Previously val != null blocked the
push because null == null is true in loose equality, so push(null)
was never called and the stream never emitted 'end'.

Change the guard from val != null to val !== undefined so that
null passes through to this.push(), which sets state.ended and
eventually emits 'end', matching documented behavior.

Fixes: #62769
PR-URL: #62803
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

[main 68b5d23131] stream: allow null as second arg in Transform callback
Author: galaxy4276 <deveungi@gmail.com>
Date: Sun Apr 19 23:13:42 2026 +0900
2 files changed, 67 insertions(+), 1 deletion(-)
create mode 100644 test/parallel/test-stream-transform-callback-null.js
✖ 68b5d2313170fe9305cf6001a55163f9e0ff3b50
✔ 0:0 no Assisted-by metadata assisted-by-is-trailer
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 11:7 Valid fixes URL. fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 12:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✖ 0:0 Commit must have a "Signed-off-by" trailer signed-off-by
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
⚠ 0:50 Title should be <= 50 columns. title-length

ℹ Please fix the commit message and try again.
Please manually ammend the commit message, by running
git commit --amend
Once commit message is fixed, finish the landing command running
git node land --continue

https://github.com/nodejs/node/actions/runs/26708502888

nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Apr 18, 2026

mcollina added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 18, 2026

ronag approved these changes Apr 18, 2026

View reviewed changes

ronag requested a review from mcollina April 18, 2026 14:44

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 18, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 18, 2026

lpinca approved these changes Apr 18, 2026

View reviewed changes

ronag approved these changes Apr 18, 2026

View reviewed changes

galaxy4276 requested review from lpinca and ronag April 18, 2026 19:35

ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 18, 2026

mcollina requested changes Apr 18, 2026

View reviewed changes

galaxy4276 requested a review from mcollina April 19, 2026 14:09

galaxy4276 force-pushed the stream/fix-transform-callback-null branch from 4aefb07 to 3040233 Compare April 19, 2026 14:23

mcollina approved these changes Apr 20, 2026

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 20, 2026

ShenHongFei mentioned this pull request May 21, 2026

in transform stream transform method, callback(null, null) doesn't end stream but this.push(null); callback() does #62769

Open

This comment was marked as resolved.

Sign in to view

ShenHongFei reviewed May 21, 2026

View reviewed changes

Comment thread lib/internal/streams/transform.js

mcollina added the commit-queue Add this label to land a pull request using GitHub Actions. label May 31, 2026

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 31, 2026

Uh oh!

Conversation

galaxy4276 commented Apr 18, 2026

What

Bug

Fix

Behavior after fix

Potential concern

Uh oh!

nodejs-github-bot commented Apr 18, 2026

Uh oh!

galaxy4276 commented Apr 18, 2026

Uh oh!

nodejs-github-bot commented Apr 18, 2026

Uh oh!

codecov Bot commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

galaxy4276 commented Apr 18, 2026

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

galaxy4276 commented Apr 19, 2026

Uh oh!

galaxy4276 commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

citgm run — results

Environment

Full citgm-all (72 modules)

Extended stream subset (17 modules not in citgm lookup)

Pattern audit

Semantic differential

Conclusion & SemVer recommendation

Alternatives considered

Raw artifacts

Notes

Uh oh!

galaxy4276 commented Apr 19, 2026

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

mcollina commented Apr 20, 2026

Uh oh!

nodejs-github-bot commented Apr 20, 2026

Uh oh!

galaxy4276 commented Apr 20, 2026

Uh oh!

trivikr commented May 21, 2026

Uh oh!

trivikr commented May 21, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

nodejs-github-bot commented May 31, 2026

Fixes: #62769 PR-URL: #62803 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov Bot commented Apr 18, 2026 •

edited

Loading

galaxy4276 commented Apr 19, 2026 •

edited

Loading

Fixes: #62769
PR-URL: #62803
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>